Skip to content

Add module-specific assertions (Doctrine, Environment, Security, Session) - #218

Closed
TavoNiievez wants to merge 1 commit into
Codeception:mainfrom
TavoNiievez:new_asserts
Closed

Add module-specific assertions (Doctrine, Environment, Security, Session)#218
TavoNiievez wants to merge 1 commit into
Codeception:mainfrom
TavoNiievez:new_asserts

Conversation

@TavoNiievez

@TavoNiievez TavoNiievez commented Aug 17, 2025

Copy link
Copy Markdown
Member

What

Adds high-level, actor-style assertions that rely on Symfony kernel/container integration and have no pure-Symfony equivalent. These belong to the see* / dontSee* family of the naming convention documented in CONTRIBUTING.md (also extended here).

Methods

  • DoctrineAssertionsTrait: seeDoctrineDatabaseIsUp, seeDoctrineSchemaIsValid, seeDoctrineProxyDirIsWritable
  • EnvironmentAssertionsTrait (new trait): seeKernelEnvironmentIs, seeDebugModeEnabled, dontSeeDebugModeEnabled, seeSymfonyVersion, seeAppEnvAndDebugMatchKernel, seeAppCacheIsWritable, seeAppLogIsWritable, seeProjectStructureIsSane, seeEnvFileIsSynchronized, seeBundleIsEnabled, seeAssetManifestExists, seeKernelCharsetIs
  • SecurityAssertionsTrait: seeFirewallIsConfigured, seeRoleInHierarchy, seeSecretCanBeResolved
  • SessionAssertionsTrait: seeSessionSavePathIsWritable

Registers EnvironmentAssertionsTrait on the Symfony module facade and documents the see* / assert* naming convention in CONTRIBUTING.md.

Tests

Unit tests added for the methods exercisable against the test app (tests/_app): Doctrine checks, the firewall check, Symfony-version/env-debug/project-dir environment checks.

The remaining checks require a full real Symfony project layout that the mini unit-test app does not provide (seeProjectStructureIsSane, seeAssetManifestExists, seeEnvFileIsSynchronized, seeSecretCanBeResolved); per the two-test-layer setup these are best covered in the companion Codeception/symfony-module-tests functional suite.

Notes

Companion to #240, which contains the Symfony-mirroring assert* / get* assertions. The two PRs touch disjoint files and can be reviewed/merged independently.

@TavoNiievez

Copy link
Copy Markdown
Member Author

Hi @ThomasLandauer @xEdelweiss ,
I'm proposing adding some new assertions to the module.

Can you take a look at the PR and give me your opinion on the new methods, please?

Comment thread src/Codeception/Module/Symfony/DoctrineAssertionsTrait.php Outdated
Comment thread src/Codeception/Module/Symfony/DoctrineAssertionsTrait.php
Comment thread src/Codeception/Module/Symfony/EnvironmentAssertionsTrait.php Outdated
Comment thread src/Codeception/Module/Symfony/EnvironmentAssertionsTrait.php
Comment thread src/Codeception/Module/Symfony/EnvironmentAssertionsTrait.php
Comment thread src/Codeception/Module/Symfony/SecurityAssertionsTrait.php Outdated
@ThomasLandauer

Copy link
Copy Markdown
Member

Well, these are many... :-)

  1. Do you have a clear picture when Codeception usually uses see vs. assert? Or at least clearer than me? ;-)
  2. Is there a reason why you're putting those Doctrine assertions in the Symfony and not in the Doctrine module?
  3. For some assertions I don't get the idea, since the outcome usually differs between TEST and PROD environment, e.g. seeDoctrineDatabaseIsUp(), seeDebugModeEnabled(). What's your reasoning here?

@xEdelweiss

Copy link
Copy Markdown
Contributor

Hi @TavoNiievez
Thanks for looping me in! Sadly, I'm not really working with Codeception/Symfony at the moment, so I don't think I can give this PR the proper attention it deserves.

@TavoNiievez
TavoNiievez force-pushed the new_asserts branch 6 times, most recently from 42f67c0 to afb9a63 Compare April 14, 2026 19:53
@TavoNiievez

TavoNiievez commented Jun 23, 2026

Copy link
Copy Markdown
Member Author

Thanks for the thorough review, @ThomasLandauer! Answers to your three higher-level questions:

  1. see vs assert — fair point. The module is uniformly see* / dontSee* / grab*, so I aligned the two outliers I'd introduced: assertSymfonyVersionseeSymfonyVersion and assertEnvFileIsSynchronizedseeEnvFileIsSynchronized. Everything new now follows the see* convention.

  2. Doctrine assertions in the Symfony module — this module already ships a DoctrineAssertionsTrait (grabRepository, seeNumRecords, grabNumRecords, …), since Symfony + Doctrine is the default stack and there's no hard dependency forcing module-doctrine. The new assertions just extend that existing trait rather than create a new home for them.

  3. Outcome differs between TEST and PROD — these assert the expected state for whatever environment the suite targets, instead of a fixed value. That's why both polarities exist (seeDebugModeEnabled / dontSeeDebugModeEnabled): you call the one that matches the environment under test. seeDoctrineDatabaseIsUp() is a connectivity smoke-check that's meaningful in any environment where the DB is expected to be reachable.

I've pushed a commit addressing the inline comments (naming + docblock clarifications) and replied in each thread.

@TavoNiievez
TavoNiievez force-pushed the new_asserts branch 2 times, most recently from 1614142 to 2826ad5 Compare June 23, 2026 20:00
@TavoNiievez TavoNiievez changed the title Add new assertions Add module-specific assertions (Doctrine, Environment, Security, Session) Jun 23, 2026
@ThomasLandauer

Copy link
Copy Markdown
Member

Looks like GitHub's "Mark conversation as resolved" button is gone, so I just gave a thumbs-up to all above line comments ;-)

For the other points:

  1. Foreword: This will remain a source of debate/confusion forever ;-)
    I think what you're saying in CONTRIBUTING.md (see for high-level, assert for low-level) makes sense; and I think somebody (maybe you after all) has also explained it like that somewhere - but I can't find that thread...
    But what you're saying here in the comment ("outliers") contradicts that, IMO.
    Long story short: I would keep assertSymfonyVersion, cause this is low-level to me.
    But if you feel better with see, then keep it -> see foreword :-)
  2. Naive question: Wouldn't it be better to just force/assume that everybody has the Doctrine module (with exactly your argument, that most Symfony users are using Doctrine anyway), instead of duplicating (half of) it here?

…ion)

Add high-level, actor-style assertions that rely on Symfony kernel and
container integration and have no pure-Symfony equivalent. They belong to
the see* / dontSee* family of the naming convention documented in
CONTRIBUTING.md, which is extended here.

- DoctrineAssertionsTrait: seeDoctrineDatabaseIsUp, seeDoctrineSchemaIsValid,
  seeDoctrineProxyDirIsWritable
- EnvironmentAssertionsTrait (new): seeKernelEnvironmentIs,
  seeDebugModeEnabled, dontSeeDebugModeEnabled, seeSymfonyVersion,
  seeAppEnvAndDebugMatchKernel, seeAppCacheIsWritable, seeAppLogIsWritable,
  seeProjectStructureIsSane, seeEnvFileIsSynchronized, seeBundleIsEnabled,
  seeAssetManifestExists, seeKernelCharsetIs
- SecurityAssertionsTrait: seeFirewallIsConfigured, seeRoleInHierarchy,
  seeSecretCanBeResolved
- SessionAssertionsTrait: seeSessionSavePathIsWritable

Register EnvironmentAssertionsTrait on the Symfony module facade and
document the see* / assert* naming convention in CONTRIBUTING.md.

Unit tests cover the methods exercisable against the test app under
tests/_app. The remaining checks need a full Symfony project layout that the
mini test app does not provide (seeProjectStructureIsSane,
seeAssetManifestExists, seeEnvFileIsSynchronized, seeSecretCanBeResolved);
per the two-layer test setup those belong in the companion
Codeception/symfony-module-tests functional suite.

Also stop the functional CI job from installing a second copy of the module
into the app: it removed codeception/module-symfony with --no-update and
then ran `composer install`, which reinstalls from the app's lockfile, so
the module source under test and a stale released copy were both
autoloaded.
@TavoNiievez

Copy link
Copy Markdown
Member Author

Closed and recreated from the same branch (same commit) to clear stale force-push entries in this timeline. The replacement is pull request 246; the review discussion on this page stays here for reference.

@ThomasLandauer

Copy link
Copy Markdown
Member

Link: #246

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants